-
Notifications
You must be signed in to change notification settings - Fork 825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for shared memory in Wasm. #702
Conversation
Adds trap for misaligned accesses.
Enable shared memory usage in simd.wast. Exclude atomic.wast on singlepass and clif.
} | ||
|
||
pub struct SharedMemoryInternal { | ||
memory: RefCell<Box<StaticMemory>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe dumb question: shared memory is always static?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we've implemented it, yes, but I can't see any reason we couldn't add SharedDynamic if we wanted to. The challenge would be ensuring that the base pointer of the dynamic memory doesn't move while we're forming and using a pointer into that memory. A naive implementation could hold a mutex lock for every memory access in the wasm program.
An efficient implementation might implement base changes by temporarily mapping the same pages at two addresses in memory so that old pointers continue to work during the move. Then, remove the old mapping such that the pointers fault. You can't use that memory again until you can prove all old pointers are gone (otherwise you break Wasm security model). For the wasm program's loads and stores, either it would need to reload the base pointer if the access faults (if we unmap the memory early), or you keep the temporary mapping around until no such pointers exist but that means the grow
operation is blocked until then too.
@@ -289,27 +282,56 @@ impl Clone for UnsharedMemory { | |||
} | |||
|
|||
pub struct SharedMemory { | |||
#[allow(dead_code)] | |||
desc: MemoryDescriptor, | |||
internal: Arc<SharedMemoryInternal>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need an internal structure? can we move all the fields from SharedMemoryInternal
into the SharedMemory
itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is the implementation of Clone for SharedMemory. Mind you, I'm suspicious of the fact that we clone memories to begin with, but I took a look and decided that was a problem for another day. The important part is that the underlying size/grow methods share one lock, which is held by SharedMemoryInternal
while the SharedMemory
can be cloned to your heart's content.
lib/llvm-backend/src/code.rs
Outdated
@@ -4395,6 +4448,2024 @@ impl FunctionCodeGenerator<CodegenError> for LLVMFunctionCodeGenerator { | |||
let res = builder.build_bitcast(res, intrinsics.i128_ty, ""); | |||
state.push1(res); | |||
} | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented out code, or they could return CodegenError
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to add a instances with shared memory test for the Rust API side.
bors r+ |
702: Add support for shared memory in Wasm. r=nlewycky a=nlewycky Add support for memory regions with the shared flag. These are part of the threads proposal. - Add new `--enable-threads` flags and pass it through to wasmparser.rs and wabt (for .wat files). - Enable `--enable-threads` when parsing spectests. Enables the shared memory region in `simd.wast`. - Adds `atomic.wast`, the spec test for the threads proposal. With the LLVM backend, all tests pass. - Removes the `runtime-core/src/memory/static_` directory since there's no need for it to include two implementations. We use the same implementation of static memory for shared and unshared static memory. - Implements the atomic instructions in the LLVM backend. However, `atomic.load` and `atomic.store` are provided **with no atomicity.** The rmw and cmpxchg instructions are sequentially consistent. - `atomic.notify` and `atomic.wait` are unimplemented. Co-authored-by: Nick Lewycky <[email protected]>
Add support for memory regions with the shared flag. These are part of the threads proposal.
--enable-threads
flags and pass it through to wasmparser.rs and wabt (for .wat files).--enable-threads
when parsing spectests. Enables the shared memory region insimd.wast
.atomic.wast
, the spec test for the threads proposal. With the LLVM backend, all tests pass.runtime-core/src/memory/static_
directory since there's no need for it to include two implementations. We use the same implementation of static memory for shared and unshared static memory.atomic.load
andatomic.store
are provided with no atomicity. The rmw and cmpxchg instructions are sequentially consistent.atomic.notify
andatomic.wait
are unimplemented.